Skip to content
This repository has been archived by the owner on Jul 16, 2023. It is now read-only.

save accepts Input::all() fixes "sometimes" validator #166

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

save accepts Input::all() fixes "sometimes" validator #166

wants to merge 3 commits into from

Conversation

tbergeron
Copy link

That validator only validates the field when it is specified in the form's data (when it's in the attributes). Ardent used the model's attributes instead of using the actual data coming from the form.

i.e. Saving a user without checking if his username is unique on every save, would simply never work.

public static $rules = array(
    'username'  => 'sometimes|required|unique:users'
);

Now we can do:

public function update($id)
    $user = User::findOrFail($id);

    // NOTE: No need to do ->fill() as ->save() now handles this part.
    if ($user->save(Input::all())) {
        // Validated!
    } else {
        return Redirect::back()->withInput()->withErrors($user->errors());
    }
}

save method now looks like this:

public function save(array $data = null,
                         array $rules = array(),
                         array $customMessages = array(),
                         array $options = array(),
                         Closure $beforeSave = null,
                         Closure $afterSave = null) {
// ...

Hope this helps somebody ;-)

@tbergeron
Update Ardent.php
27b3646
@tbergeron
save accepts Input::all() fixes "sometimes"
674620a 
->save($data, $rules, $customMessages, $beforeSave, $afterSave); that way the "sometimes" validator will work.
@tbergeron
Update README.md
cfb6e59
@igorsantos07
Copy link
Member

Sorry, I didn't understand the point on this PR. I haven't used Ardent for quite some time, but in my understanding, an AR model should always validate it's internal fields, unrelated to form data. If you want to validate what's on the form you pull it inside the model first, isn't it?

Furthermore, I think that although it makes sense to use the said $data argument as the first one, that's a huge breaking change in comparison with the current library usage. This would probably not be implemented alone, as I would need to release a new major tag only for this change, and people would need to update all their codebase just because of this.

@jayremias
Copy link

I'm having some trouble related with "sometimes". I'm trying to update a user, but without change the password field. So the Validator return an error, because the password field is already set on the model attributes but the password confirmation isn't. So I get an error. Maybe to get it working, unset unchanged attributes or validate attributes with changes. Any other options with the current library?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
question Review RFC
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tbergeron @igorsantos07 @jayremias